Thanks for working on this and kudos for making it work!

The major issue causing the CI failures is usage of std which needs to be guarded.

I still need to play with the code, but I left some nitpicks about dosctrings and comments.

Reviewable status: 0 of 11 files reviewed, 25 unresolved discussions

aya-obj/src/lib.rs line 79 at r1 (raw file):

pub mod btf;
/// ksym implementation

Let's remove this. If you want to have a top-level documentation of the module, add it in the top of the module file with !//.

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

use std::{

This is the reason why CI is failing. The aya-obj crate supports no_std, so everything requiring std needs to be guarded with #[cfg(feature = "std")]. See the other modules

I still need to take a deeper look, but given that this whole ksym detection/sanitization mechanism clearly requires allocations and string operations, we could probably just guard the whole module with #[cfg(feature = "std")].

Another option would be trying to make it no_std, but I don't think it's possible.

aya-obj/src/extern_types.rs line 58 at r1 (raw file):

            }

            // Dispatch to appropriate resolver

I would remove all the comments in this function except that one.

aya-obj/src/extern_types.rs line 99 at r1 (raw file):

    fn resolve_kallsyms(&mut self) -> std::result::Result<(), KsymResolveError> {
        use std::{

Move the imports up.

aya-obj/src/extern_types.rs line 122 at r1 (raw file):

        let file =
            File::open("/proc/kallsyms").map_err(|e| KsymResolveError::KallsymsReadError {
                error: e.to_string(),

We shouldn't include other errors with string.thiserror can wrap other error types inside your own one.

aya-obj/src/extern_types.rs line 227 at r1 (raw file):

    /// Internal: Resolve a single extern variable
    /// Returns Some(btf_id) on success, None for weak externs not found

Code snippet:

/// Resolves a single exterm variable. Returns BTF ID if found, otherwise
/// returns `None`.

aya/src/bpf.rs line 97 at r1 (raw file):

}

pub(crate) static KERNEL_BTF: LazyLock<Option<Btf>> = LazyLock::new(|| match Btf::from_sys_fs() {

btw, I still need to play with the code myself, but I don't like having it as a global variable. If it's really difficult to pass it across functions, we could make it a part of some struct and write methods.

aya/src/bpf.rs line 1195 at r1 (raw file):

    /// Error resolving extern kernel symbols (functions and variables)
    #[error("kernel symbol resolution error: {0}")]
    KsymResolve(#[from] aya_obj::KsymResolveError),

Usually we are trying to keep the error variants in EbpfError very general, and treat each variant as a category of errors. Given that these two are about ksyms, I would try to pack them in one error type first (let's stay, KsymError), then include it here.

test/integration-test/bpf/ksysm.bpf.c line 18 at r1 (raw file):

extern void bpf_rcu_read_lock(void) __attribute__((section(".ksyms")));
extern void bpf_rcu_read_unlock(void) __attribute__((section(".ksyms")));

Would be nice to add a similar Rust eBPF program.

aya-obj/src/btf/btf.rs line 819 at r1 (raw file):

    /// This modifies extern functions and variables to make them acceptable to the kernel:
    /// - Functions: Change linkage to GLOBAL, fix param names, replace with dummy var in datasec
    /// - Variables: Change linkage to GLOBAL_ALLOCATED, replace type with int

Code snippet:

/// Fixes up BTF for `.ksyms` datasec entries containing extern kernel symbol and
/// makes it acceptable by the kernel:
///
/// * Changes linkage of extern functions to `GLOBAL`, fixes parameter names, injects
///   a dummy variable representing them in datasec.
/// * Changes linkage of extern variables to `GLOBAL_ALLOCATED`, replaces their type
///   with `int`.

aya-obj/src/btf/btf.rs line 825 at r1 (raw file):

        dummy_var_id: Option<u32>,
    ) -> Result<(), BtfError> {
        // Get dummy var info and int type ID upfront

I would try to make this comment more descriptive (for the whole let ... else block`) and remove all the others.

aya-obj/src/relocation.rs line 184 at r1 (raw file):

    pub fn relocate_externs(&mut self) -> Result<(), EbpfRelocationError> {
        for (name, extern_desc) in &self.externs.externs {
            eprintln!(

Use debug!, instead of eprintln!

aya-obj/src/relocation.rs line 246 at r1 (raw file):

fn relocate_externs<'a, I: Iterator<Item = &'a Relocation>>(
    fun: &mut Function, // ← Different first parameter (not &mut self)

Remove this comment, it brings no value. It's not even a method, so why would it have self?

aya-obj/src/obj.rs line 872 at r1 (raw file):

        };

        info!("Found/created int type_id: {}", int_type_id);

I would make it debug! and print it only if we create the int type.

aya-obj/src/obj.rs line 890 at r1 (raw file):

        // After creating dummy_var
        info!("Created dummy_var type_id: {}", dummy_var_id);

debug!

aya-obj/src/obj.rs line 898 at r1 (raw file):

    }

    /// Collect extern kernel symbols from BTF DATASEC entries

Function/method documentation should be written using the 3rd person, so starting from "Collects". And if you use full sentences, finish them with ..

Let's also be consistent with capitalizing (or not capitalizing) names. We either write "DATASEC" or "datasec" everywhere. I'm leaning towards the latter, since it's just a convenient shortened form of "data section", not really a proper name.

Code snippet:

/// Collects extern kernel symbols from BTF datasec entries.

aya-obj/src/obj.rs line 900 at r1 (raw file):

    /// Collect extern kernel symbols from BTF DATASEC entries
    fn collect_ksyms_from_btf(&mut self) -> Result<(), ParseError> {
        // Find .ksyms DATASEC

Remove all these comments, the code is self-explanatory enough.

aya-obj/src/obj.rs line 927 at r1 (raw file):

    }

    /// Find .ksyms DATASEC in BTF, returns (id, datasec) if found

Code snippet:

/// Searches for the `.ksyms` datasec in BTF, returns it if found.

aya-obj/src/obj.rs line 945 at r1 (raw file):

    }

    /// Check if datasec contains any functions

Code snippet:

/// Checks if datasec dontains any functions.

aya-obj/src/obj.rs line 954 at r1 (raw file):

    }

    /// Collect extern descriptors from datasec entries

Code snippet:

/// Collects extern descriptors from datasec entries.

aya-obj/src/obj.rs line 961 at r1 (raw file):

        for entry in &datasec.entries {
            let Some(extern_desc) = self.process_datasec_entry(btf, entry)? else {
                continue; // Skip non-extern entries

Remove this comment.

aya-obj/src/obj.rs line 970 at r1 (raw file):

    }

    /// Process a single datasec entry, returning ExternDesc if it's an extern

Aside from what I mentioned earlier, try to use ``` for type names. If the types are inside the same crate, you can even link them like:

Code snippet:

/// Processes a single datasec entry, returns [`ExternDesc`] if it's an extern.

aya-obj/src/obj.rs line 978 at r1 (raw file):

        let btf_type = btf.type_by_id(entry.btf_type)?;

        // Extract extern info based on type

Remove these overly explicit comments.

aya-obj/src/obj.rs line 1017 at r1 (raw file):

    }

    /// Apply BTF fixups for .ksyms DATASEC

Code snippet:

/// Applies BTF fixups for `.ksyms` datasec.

aya-obj/src/obj.rs line 1026 at r1 (raw file):

    }

    /// Helper to find symbol by name

Don't start function/method documentation with "Helper", use the 3rd person and explain what it does.

Code snippet:

/// Return a symbol with the given `name` from the symbol table.

Some more comments about code organization, the kind of comments you were asking for. 😄

Reviewable status: 0 of 15 files reviewed, 36 unresolved discussions (waiting on @vadorovsky)

aya/src/bpf.rs line 97 at r1 (raw file):

removed, now we are using already parsed kernel btf.

aya/src/bpf.rs line 1195 at r1 (raw file):

Done.

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

Done.

aya-obj/src/extern_types.rs line 58 at r1 (raw file):

Done.

aya-obj/src/extern_types.rs line 99 at r1 (raw file):

would not it be better to be stayed in the function scope since this is only enabled with std feature

aya-obj/src/extern_types.rs line 122 at r1 (raw file):

Done.

aya-obj/src/lib.rs line 79 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 872 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 890 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 898 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 900 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 961 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 970 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 978 at r1 (raw file):

Done.

aya-obj/src/obj.rs line 1026 at r1 (raw file):

Done.

aya-obj/src/relocation.rs line 184 at r1 (raw file):

Done.

aya-obj/src/relocation.rs line 246 at r1 (raw file):

Done.

aya-obj/src/btf/btf.rs line 825 at r1 (raw file):

Done.

test/integration-test/bpf/ksysm.bpf.c line 18 at r1 (raw file):

I added a similar rust program, but extern definitions debug information is not emitted by rustc so that the proper btf is generated by llvm. We need a thread in discord regarding this.

aya-obj/src/extern_types.rs line 227 at r1 (raw file):

    /// Internal: Resolve a single extern variable
    /// Returns Some(btf_id) on success, None for weak externs not found

Done.

aya-obj/src/btf/btf.rs line 819 at r1 (raw file):

    /// This modifies extern functions and variables to make them acceptable to the kernel:
    /// - Functions: Change linkage to GLOBAL, fix param names, replace with dummy var in datasec
    /// - Variables: Change linkage to GLOBAL_ALLOCATED, replace type with int

Done.

aya-obj/src/obj.rs line 927 at r1 (raw file):

    }

    /// Find .ksyms DATASEC in BTF, returns (id, datasec) if found

Done.

aya-obj/src/obj.rs line 945 at r1 (raw file):

    }

    /// Check if datasec contains any functions

Done.

aya-obj/src/obj.rs line 954 at r1 (raw file):

    }

    /// Collect extern descriptors from datasec entries

Done.

aya-obj/src/obj.rs line 1017 at r1 (raw file):

    }

    /// Apply BTF fixups for .ksyms DATASEC

Done.